-
-
Notifications
You must be signed in to change notification settings - Fork 4.3k
fix: Allows disabling auto mode, as well as improves support for custom node pools #3513
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: Allows disabling auto mode, as well as improves support for custom node pools #3513
Conversation
…and storage_config
When disabling auto mode, fixes the apply-time error: ``` │ Error: updating EKS Cluster (ex-eks-auto-mode) compute config: operation error EKS: UpdateClusterConfig, https response error StatusCode: 400, RequestID: f895a875-7bbf-4003-92e0-3aca4bc9d415, InvalidParameterException: When Compute Config nodeRoleArn is null or empty, nodePool list cannot be populated. ```
Fixes the apply time error: ``` │ Error: updating EKS Cluster (ex-eks-auto-mode) compute config: operation error EKS: UpdateClusterConfig, https response error StatusCode: 400, RequestID: 93002327-da4d-47e2-8389-518b03e8aa60, InvalidParameterException: When Compute Config nodeRoleArn is not null or empty, nodePool value(s) must be provided. ```
@bryantbiggs Yeah, I'm finding that PR isn't required to resolve the problem. But it's also not entirely clear to me what that PR is trying to resolve... Without a clear reproduction case, it feels a little like they are trying to do something that the AWS API doesn't actually allow? |
Try it by applying the auto mode example from this PR, and then set |
ce7df6d
to
3531c93
Compare
|
||
enable_cluster_creator_admin_permissions = true | ||
|
||
compute_config = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#3514 does what I believe you are trying to do. this does not work as its written and throws the following error
its also not very intuitive, in my opinion, to know that you need to set node_pools = []
just to get the IAM resources required for using only custom node pools
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've applied and updated and destroyed my config a dozen times with various inputs. What exact combination of configs and workflow generated that error? If you look at the commit message for 076e62d
(#3513), you'll see that is the exact issue I was addressing (or attempting to...).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because I set the default compute_config.node_pools = []
, you do not actually need to set that in config to enable auto mode and use the node iam role for a custom node pool. You can pass in only compute.config.enabled = true
and it will do the right thing.
This issue has been resolved in version 21.3.0 🎉 |
Description
This change allows EKS Auto Mode to be disabled, after being enabled. It also allows the user to remove and re-add the built-in node pools, without recreating the entire cluster (this part requires aws provider v6.13.0, I can bump the module min version if desired).
Motivation and Context
Primary motivation is to be able to sucessfully disable auto mode. Currently this module can create a cluster with auto mode enabled, but cannot disable auto mode. This is because of a logic error in
elastic_load_balancing
andstorage_config
. Instead of removing the blocks entirely from the config as it currently does, the API requires that the blocks be present but setenabled = false
.Also, when disabling auto mode, the API requires that
compute_config.node_pools
be empty. Instead of requiring the user to set bothcompute_config.enabled
andcompute_config.node_pools
, this changeset coerces the node pools to an empty list when the user sendsenabled
= false`.And finally, when the user enables auto mode but wants to use custom node pools, then
compute_config.node_role_arn
needs to benull
.Fixes #3273
Breaking Changes
None
How Has This Been Tested?
examples/*
to demonstrate and validate my change(s)examples/*
projectspre-commit run -a
on my pull request